Bound the display handle enumeration and stop stranding vibrance (#138) - #158

Three stability defects, all reachable without a hand-edited settings file. Split into two commits so the fixes can be taken without the testing convention — see "About the second commit" below.

1. Unbounded display handle enumeration — closes #138

EnumerateDisplayHandles() loops until enumerateNvidiaDisplayHandle returns -1, and nothing caps the index:

for (int i = 0, displayHandle = 0; displayHandle != -1; i++)

#138 reports "Extreme CPU usage when no dedicated GPU connected" from someone whose NVIDIA GPU lives in a Thunderbolt enclosure. Unplug it and nvapi.dll is still installed, but the enumeration never terminates. InitializeProxy() then never returns, so isInitialized is never set and OnWinEventHook is never subscribed — and in this x86 process the unbounded List<int> exhausts the address space, so the OutOfMemoryException gets caught by the constructor and surfaces as the "failed to initialize" dialog.

The loop is now bounded at a new NvapiMaxDisplays, derived from the NvapiMaxPhysicalGpus constant this class already uses to size its GPU handle arrays. No nvapi.h is vendored in the repo, so the comment does not claim to match NvAPI's own ceiling — it deliberately over-approximates, which is what a bound that must never truncate a real display should do.

Handles are also deduplicated: a driver stuck returning the same handle would otherwise fill the list with copies, each costing its own setDVCLevel call on the restore path.

To be precise about what the dedupe is and is not — it is a latent cost, not the cause of #138. Pre-fix the restore path was unreachable for the reason above, so those duplicates were never walked.

2. Removing the last game strands vibrance

OnWinEventHook wraps its entire body in if (_applicationSettings.Count > 0) — the else restore branch included — in both proxies.

Remove the last saved game while its game holds the foreground and the vibrance level and the resolution change are both stranded, with no way back short of restarting.

The match is now computed conditionally so the restore branch is always reachable. The match expression itself is unchanged.

3. Double-clicking empty space crashes the app

listApplications_DoubleClick indexes SelectedItems[0] with no count check. ListView raises DoubleClick for the whole control, empty space included, where the indexer throws ArgumentOutOfRangeException out of a UI event handler — nothing on the path catches it, and no Application.ThreadException handler is installed.

Verification

The apply branches are untouched. Most of the diff in item 2 is a mechanical dedent. Stripping comments and normalising whitespace, the apply and restore bodies in both proxies are identical to master:

NVIDIA APPLY   identical after normalization: True
NVIDIA RESTORE identical after normalization: True
AMD    APPLY   identical after normalization: True
AMD    RESTORE identical after normalization: True

Warnings unchanged. master builds with 2 warnings (ResolutionModeWrapper.cs:8 CS0659, WinEventHook.cs:202 CS0168); this branch builds with the same 2. Neither file is touched here. Release and Debug x86 both build with 0 errors.

Against real hardware. Driving the real vibranceDLL.dll on a three-monitor machine, the enumeration returns three distinct handles and terminates at index 3 — nowhere near the bound, and the dedupe is a no-op there.

About the second commit

The repo has no test project, so the second commit adds a --selftest-stability flag and a StabilityFixture covering all three fixes — six checks, run without a GPU or the native DLL by driving the extracted loop with a stub delegate.

This introduces a convention that doesn't exist here yet. It's a separate commit specifically so you can drop it and take only the fixes if you'd rather not adopt it.

It is real evidence rather than decoration: swapping master's AmdDynamicVibranceProxy.cs back in makes the restore check fail (5/6, restore call count 0), which is exactly the pre-fix behaviour.

[PASS] an enumerator that never returns -1 stops at NvapiMaxDisplays (256), got 256
[PASS] a driver that always returns the same handle yields exactly one entry, not 256 copies of it
[PASS] duplicates are dropped wherever they recur in the sequence
[PASS] an enumerator that returns -1 immediately yields a non-null, empty list
[PASS] the restore call ran once (pre-fix, Count > 0 gated the whole handler and it never ran at all)
[PASS] SetSaturationOnAllDisplays' last call used the matched setting's IngameLevel (77), not the Windows default
PASSED 6/6

Known limits

• Extreme CPU usage when no dedicated GPU connected #138's own symptom is not reproducible on my hardware — it needs a machine with no NVIDIA GPU attached. The bounded and deduped logic is proven correct in isolation; the end-to-end fix on the reporter's configuration is not. @OneBiteAidan would be the person to confirm.
• NVIDIA's restore branch has no automated coverage. It calls straight into the prebuilt native DLL, which isn't loadable from a self test. AMD's equivalent is covered, and the two were shown structurally identical.
• With zero configured games the restore branch now runs on every foreground change. This is the same work the no-match case already did on every alt-tab, but it is new for a user with an empty list.
• The fixture's restore check depends on the foreground window not changing mid-test. Measured at 0 failures in 90,000+ passive iterations and ~0.003% under deliberately engineered contention; it reports [SKIP] rather than [FAIL] if it happens.

Happy to drop the fixture commit, split this into three PRs, or adjust anything else you'd prefer.